lib: replace string concatenation with template #16923
Conversation
gireeshpunathil
added
the
code-and-learn
lib/_http_client.js
Outdated
| @@ -116,7 +116,7 @@ function ClientRequest(options, cb) { | |||
|
|
|||
| var path; | |||
| if (options.path) { | |||
| path = '' + options.path; | |||
| path = `${options.path}`; | |||
There was a problem hiding this comment.
I wonder if this could just be String(options.path).
There was a problem hiding this comment.
@cjihrig '' + foo seems pretty idiomatic at this point, perhaps due to performance. I'm not sure if that's a case of CrankshaftScript though.
¯\(ツ)/¯
There was a problem hiding this comment.
'' + foo is what is being removed here though.
There was a problem hiding this comment.
PR author and committer are not the same - probably worked on the same system with clobbered git config parameters. @Trott - please advise how to rectify it.
|
@gireeshpunathil We can change the author when we land this. Just let me know what the correct name and email address are. @VijiKannan if you prefer, you could also try to do the same change from a system where |
VijiKannan
force-pushed
the
nodepr
branch
3 times, most recently
from
df5ece3 to
17fec4b
Compare
|
I have addressed the comments, and also used the correct git config. Please have a look, thanks! |
|
Inconclusive CI results (failed in smartos and raspbian weezy), trying once again: https://ci.nodejs.org/job/node-test-pull-request/11496/ |
|
Landed in a0aff57, thank you for your contribution! 🎉 |
PR-URL: #16923 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: #16923 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: #16923 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: #16923 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
This is my first contribution to node project